Skip to content

Unified: Bazel downloaded toolchain on macOS - #22339

Merged
jketema merged 2 commits into
github:mainfrom
jketema:jketema/unified-mac-download
Aug 14, 2026
Merged

Unified: Bazel downloaded toolchain on macOS#22339
jketema merged 2 commits into
github:mainfrom
jketema:jketema/unified-mac-download

Conversation

@jketema

@jketema jketema commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Note that this still depends on the Xcode command line tools being present.

@jketema
jketema force-pushed the jketema/unified-mac-download branch from cf53fd4 to 8f5a8db Compare August 13, 2026 15:38
@jketema
jketema marked this pull request as ready for review August 14, 2026 08:47
@jketema
jketema requested review from a team as code owners August 14, 2026 08:47
@jketema
jketema requested review from redsun82 and a balanced review from Copilot August 14, 2026 08:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Switches macOS Swift builds to a pinned, downloaded rules_swift toolchain while continuing to use the locally installed macOS SDK.

Changes:

  • Adds a patched rules_swift registry version with macOS toolchain support.
  • Registers the downloaded macOS toolchain and removes the Xcode transition.
  • Updates build definitions and documentation.
Show a summary per file
File Description
MODULE.bazel Selects and registers the patched toolchain.
unified/swift-syntax-rs/BUILD.bazel Uses swift_library directly.
unified/swift-syntax-rs/README.md Documents downloaded macOS tooling.
unified/swift-syntax-rs/xcode_transition.bzl Removes the obsolete Xcode transition.
misc/bazel/registry/modules/rules_swift/metadata.json Registers the custom version.
misc/bazel/registry/modules/rules_swift/4.0.0-rc5-codeql.1/MODULE.bazel Defines custom module metadata and dependencies.
misc/bazel/registry/modules/rules_swift/4.0.0-rc5-codeql.1/source.json Configures the upstream archive and patch.
misc/bazel/registry/modules/rules_swift/4.0.0-rc5-codeql.1/patches/register_downloaded_macos_toolchain.patch Adds downloaded macOS toolchain configuration.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Balanced

strip_prefix = filename.removesuffix(".tar.gz"),
)

+ macos_sdkroot = _run(repository_ctx, ["xcrun", "--sdk", "macosx", "--show-sdk-path"]) if repository_ctx.attr.platform == "xcode" else ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair. Addressed in fe0e622. Note that Copilot still highlights the following problems with the fix:

  • watch(macos_sdkroot)  does not observe changes inside or symlink retargeting, so it won’t reliably detect  xcode-select  changes.
  • local=True  only refreshes on Bazel server restart and re-extracts the entire Swift package, which is expensive.

Recommended design: separate lightweight SDK discovery into its own local/configure repository while keeping the downloaded Swift toolchain cacheable. Also document that persistent  xcode-select  changes require  bazel shutdown  or configuration refresh.

This is fair, but follow-up work is to get rid completely of the host command line tools dependency, so I don't want to spend too much time on making this perfect.

@redsun82 redsun82 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@jketema
jketema merged commit 6d0f21d into github:main Aug 14, 2026
82 checks passed
@jketema
jketema deleted the jketema/unified-mac-download branch August 14, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants